fix(cash): keep a module's authority over its own pending withdrawal after de-whitelisting - #350

Abstract

Modules such as StockWithdrawModule (and PaymentLinkModule, #349) move a safe's funds by asking CashModule for a withdrawal with the module as recipient. Today, if governance removes such a module from CashModule's withdraw-requester list or the DataProvider whitelist while one of its withdrawals is pending, CashModule stops treating that withdrawal as the module's. This PR records which pending withdrawals a module requested, so removing a module blocks only its new requests.

How module withdrawals work

flowchart LR
  M["Module<br/>(StockWithdraw, PaymentLink)"] -->|"requestWithdrawalByModule"| CM["CashModule<br/>pending withdrawal"]
  CM -->|"after the delay:<br/>process"| M
  X["Card spend, preLiquidate,<br/>owner's own withdrawal"] -->|"cancelOldWithdrawal"| CM
  CM -->|"cancelBridgeByCashModule<br/>callback"| M

A safe has at most one pending withdrawal. A module requests it, CashModule holds it for the withdrawal delay, and then it is processed into the module. A card spend, a preLiquidate, or the owners' own withdrawal cancels it and calls the module back so the module can update its records.

What goes wrong today after a module is removed while its withdrawal is pending:

• Anyone can processWithdrawal the hold into the module, skipping the module's bookkeeping. For PaymentLinkModule, the sender's funds arrive as unrecorded surplus.
• The module can no longer cancel its own hold.
• cancelOldWithdrawal skips the callback, so the module's record goes stale.

Until this PR is deployed, #349 relies on a governance rule: never de-whitelist PaymentLinkModule while a link is Requested.

What changes

• Record: requestWithdrawalByModule sets SafeCashConfig.withdrawalRequestedByModule. Every path that deletes the pending request, whether processing or cancelling, clears it.
• Authority: CashLendLib.isModuleWithdrawal(safe) holds when the record is set or the recipient is still on the requester list. Processing, the owners' cancelWithdrawal and cancelWithdrawalByModule all use it, so:
  • only the recipient module can process its recorded withdrawal;
  • the module can still cancel it;
  • owners still can't cancel a module's withdrawal directly. They cancel through the module, replace it with their own withdrawal, or a spend cancels it. For StockWithdrawModule after a DataProvider removal, the owner-signed cancel can't get a nonce, so owners use cancelExpiredWithdrawal or a replacement withdrawal (tested).
• Cancel callback:
  • A module still on both lists gets the callback as before, and a revert still reverts the cancel.
  • A removed module with a recorded request gets _notifyRemovedModule: a call capped at 200k gas whose result is ignored. A broken or retired module therefore can't block spends, preLiquidate or owner replacements.
• Older requests: requests made before the upgrade carry no record and keep today's rules until they clear.

Merge order with #349

#349's tests pin today's CashModule behavior, so they fail against this change. claude/plm-tests-on-cashmodule-fix is #349 with this branch merged in and its tests updated (101 passed). Whichever of the two PRs merges second carries those test changes.

Storage layout

One bool is appended at the end of SafeCashConfig, a mapping value, and packs into its existing last slot. The struct size is unchanged, and WithdrawalRequest and SafeData (the getData ABI) are untouched. CashModuleStorage itself is unchanged at 544 bytes, including after the rebase onto master's moduleWithdrawalDelayConfig. test_record_packsIntoSafeCashConfigTailSlot pins the slot and offset.

 t_struct(SafeCashConfig) numberOfBytes=576
   slot 17 off  0 incomingMode t_enum(Mode)
   slot 17 off  1 lendOptedOut t_bool
   slot 17 off  2 lendOptOutFinalizeTime t_uint96
   slot 17 off 14 usesLendGateway t_bool
+  slot 17 off 15 withdrawalRequestedByModule t_bool

Upgrade path (not in this PR)

• Deploy: a new CashLendLib (a linked library), then new CashModuleCore and CashModuleSetters implementations through CREATE3 with fresh salts, both linked to that library.
• Upgrade: one 2-day upgrade-timelock batch with upgradeToAndCall(core, "") and setCashModuleSettersAddress(setters), then a verify script. Both must land together: a mixed old and new pair could create requests without the record or leave a stale one.
• Before removing a module: wait until its pre-upgrade holds have cleared, as the existing preflight requires.
• Sizes against master: CashModuleCore is 24,058 B (518 B margin), and CashModuleSetters is 23,742 B (834 B margin).
• Skipped checks: test_verifyBytecode_CashModuleCore and _CashModuleSetters in VerifyOPMainnet.t.sol are skipped until the upgrade is live.

CI

Every check passes except Run Tests (deployed), which fails only on test_verifyBytecode_TopUpDest, exactly as on master. On 2026-10-22 the TopUpDest proxy on Optimism was upgraded to code from #331, which hasn't merged, so master's TopUpDest.sol no longer matches. The check passes once #331 lands.

Follow-ups (not in this PR)

• A StockWithdrawModule reconcile, so a missed callback can't strand an order.
• Deploy and verify scripts for the CashModule upgrade.
• After the upgrade is live: re-enable the two VerifyOPMainnet checks, and scope [PAY-1170] Add PaymentLinkModule and deploy scripts #349's governance rule to pre-upgrade holds.

🤖 Generated with Claude Code

^{Need help on this PR? Tag @codesmith-bot with what you need.}